Strip basedirs from the compiler arguments too - #2840
Merged
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2840 +/- ##
==========================================
+ Coverage 76.22% 76.30% +0.07%
==========================================
Files 72 72
Lines 39890 40027 +137
==========================================
+ Hits 30407 30541 +134
- Misses 9483 9486 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
|
Hi @avikivity , Thanks for your PR. Please, fix the CI build |
avikivity
force-pushed
the
basedirs-arguments
branch
from
September 8, 2026 18:44
53813b1 to
6aeafcf
Compare
Contributor
Author
|
Update: apply cargo fmt |
avikivity
force-pushed
the
basedirs-arguments
branch
from
September 8, 2026 18:54
6aeafcf to
35175d2
Compare
Contributor
Author
|
Update: increase test coverage |
sylvestre
reviewed
Sep 12, 2026
| Cow::Owned(result) | ||
| } | ||
|
|
||
| /// Strip the base directories from a single compiler argument, for hashing. |
Collaborator
There was a problem hiding this comment.
do we really need such a long comment?
Contributor
Author
There was a problem hiding this comment.
It is rather long, but I think the details are not obvious here.
It's easy to skip over an overlong comment, but not easy to reverse-engineer the details when the comment isn't there.
Collaborator
|
please add an integration test / end-to-end that should that it is fixed. |
avikivity
force-pushed
the
basedirs-arguments
branch
from
September 12, 2026 16:42
35175d2 to
24b073f
Compare
Contributor
Author
Done. |
Contributor
Author
|
Waiting for #2853 |
sylvestre
force-pushed
the
basedirs-arguments
branch
from
September 14, 2026 08:55
24b073f to
24ca766
Compare
SCCACHE_BASEDIRS strips the base directories from the preprocessed source, but the compiler arguments are hashed verbatim. That leaves out the one thing that most reliably ties a cache entry to a single checkout: a flag that names the tree it is building. -ffile-prefix-map=/home/user/project=. The flag exists precisely so the object file does not depend on where the tree lives - every path it records is rewritten to `.` - so two checkouts of the same commit produce identical objects. They just never agree on the hash, because the flag itself spells the path out, and so they never share an entry. Which is the case basedirs is for. Strip basedirs from each argument before hashing it, at the places where an argument is expected to spell a pathname, and only there: the whole argument, as in a source file or the path of a separated option; the value of an option written with an `=`, which covers either half of a prefix map; and the value glued to a short option such as -I. A basedir appearing anywhere else is left alone, so -DROOT="/home/user/project", which the compiler bakes into the output verbatim, still counts towards the hash. Basedirs carry a trailing slash so that they only match whole path components; in an argument a component can also end at the `=` of a prefix map, or at the end of the argument, so accept those two as well. A sibling /home/user/project-docs is still not a match. The longest basedir wins. Bump CACHE_VERSION and the preprocessor cache's FORMAT_VERSION, since the arguments now reach the hash as bytes rather than through OsString's Hash impl. An integration test is added to tests/integration.
avikivity
force-pushed
the
basedirs-arguments
branch
from
September 14, 2026 09:07
24ca766 to
c7c9894
Compare
Contributor
Author
|
Update: added missing file-prefix-map integration test script |
Collaborator
|
looks good, thanks for the test |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SCCACHE_BASEDIRS strips the base directories from the preprocessed source, but the compiler arguments are hashed verbatim. That leaves out the one thing that most reliably ties a cache entry to a single checkout: a flag that names the tree it is building.
-ffile-prefix-map=/home/user/project=.
The flag exists precisely so the object file does not depend on where the tree lives - every path it records is rewritten to
.- so two checkouts of the same commit produce identical objects. They just never agree on the hash, because the flag itself spells the path out, and so they never share an entry. Which is the case basedirs is for.Strip basedirs from each argument before hashing it, at the places where an argument is expected to spell a pathname, and only there: the whole argument, as in a source file or the path of a separated option; the value of an option written with an
=, which covers either half of a prefix map; and the value glued to a short option such as -I. A basedir appearing anywhere else is left alone, so -DROOT="/home/user/project", which the compiler bakes into the output verbatim, still counts towards the hash.Basedirs carry a trailing slash so that they only match whole path components; in an argument a component can also end at the
=of a prefix map, or at the end of the argument, so accept those two as well. A sibling /home/user/project-docs is still not a match. The longest basedir wins.Bump CACHE_VERSION and the preprocessor cache's FORMAT_VERSION, since the arguments now reach the hash as bytes rather than through OsString's Hash impl.